Skip to content

gh-141510: Use frozendict in the stdlib#144909

Merged
vstinner merged 6 commits intopython:mainfrom
vstinner:frozendict_stdlib
Mar 6, 2026
Merged

gh-141510: Use frozendict in the stdlib#144909
vstinner merged 6 commits intopython:mainfrom
vstinner:frozendict_stdlib

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Feb 17, 2026

Co-authored-by: Donghee Na <donghee.na@python.org>
@vstinner
Copy link
Member Author

This PR only changes private names, so it should not impact stdlib users.

@vstinner
Copy link
Member Author

If this change is merged, I will propose a follow-up PR which modify public names.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One optional suggestion in typing.py

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for ssl and a suggestion for opcode

Lib/opcode.py Outdated
_cache_format = {
"LOAD_GLOBAL": {
_cache_format = frozendict(
LOAD_GLOBAL={
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values of LOAD_GLOBAL can also be a frozendict (that is, we can have nested frozendicts).

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corona10
Copy link
Member

The SC does not allow mechanically replacing all uses of dict. Please wait for reviews from the code owners of each module.

@JelleZijlstra
Copy link
Member

What's the benefit of changing this in e.g. typing.py? It might break some users who modify this dict. That's not a supported use case, but we shouldn't go out of our way to break users unless there's some benefit.

@vstinner
Copy link
Member Author

@JelleZijlstra:

What's the benefit of changing this in e.g. typing.py? It might break some users who modify this dict. That's not a supported use case, but we shouldn't go out of our way to break users unless there's some benefit.

The purpose of this change is to prevent dictionary modifications, especially if the modification is done by mistake.

@vstinner
Copy link
Member Author

@JelleZijlstra: Do you prefer that I revert the typing._PROTO_ALLOWLIST change?

@picnixz
Copy link
Member

picnixz commented Feb 21, 2026

While the variables in stringprep are public, note that the file is autogenerated:

# This file is generated by mkstringprep.py. DO NOT EDIT.

And the docs clearly state:

As a result, these tables are exposed as functions, not as data structures

So, I really think we should consider any direct use or mutation of those variables as really a bad idea and I don't think we should even consider code that messes with that as being legitimate.

@vstinner
Copy link
Member Author

While the variables in stringprep are public, note that the file is autogenerated

I revert the stringprep change to restrict this change to private variables. Changing private variables type should be less controversial than changing public variables type.

@vstinner
Copy link
Member Author

vstinner commented Mar 2, 2026

I reverted the typing.py changes. I plan to merge this PR at the end of the week, unless there is a strong reason to not make these changes.

@vstinner
Copy link
Member Author

vstinner commented Mar 5, 2026

If someone really want/need to modify one of the frozendict, it remains possible. Example: platform._ver_stages |= frozendict(final=60)

$ ./python
>>> import platform
>>> platform._ver_stages
frozendict({'dev': 10, 'alpha': 20, 'a': 20, 'beta': 30, ..., 'p': 200})

>>> platform._ver_stages['final']=60
...
TypeError: 'frozendict' object does not support item assignment

>>> platform._ver_stages |= frozendict(final=60)
>>> platform._ver_stages
frozendict({'dev': 10, 'alpha': 20, 'a': 20, 'beta': 30, ..., 'p': 200, 'final': 60})

The purpose of this change is more to catch dictionaries modified by accident.

@vstinner vstinner merged commit 349639c into python:main Mar 6, 2026
48 checks passed
@vstinner vstinner deleted the frozendict_stdlib branch March 6, 2026 09:25
@vstinner
Copy link
Member Author

vstinner commented Mar 6, 2026

I merged my PR, thanks for reviews. If someone comes up with an concrete use case to modify one of these private attributes, we can revert the affected attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants